Ensure Content is read if ContentLength Header is null #27
Conversation
| @@ -651,7 +651,7 @@ void IRestCustomizable.RemoveCustomization(RestRequestCustomization customizatio | |||
| } | |||
|
|
|||
| // See if we have a JSON error to get some more details from | |||
| if (response.Content.Headers.ContentLength is not > 0) | |||
| if (response.Content.Headers.ContentLength == 0) | |||
There was a problem hiding this comment.
Somewhat of a nitpick, but shouldn't this be is 0 for style/consistency
There was a problem hiding this comment.
Up to you, I was matching line 589 further up in the file that uses the same pattern. Whatever is decided on though I think those 2 checks should probably match.
There was a problem hiding this comment.
Yeah, stick to the same as the other check - we can do a pass over constants + pattern matching later.
|
I guess I missed the analyzer tests when I was running it locally. The tests seem like they were failing because of this issue dotnet/roslyn-sdk#1099 (comment), changing it to the non *.Xunit packages and switching to DefaultVerifier like it suggests fixes the immediate issues but results in the following error: Where the 4 extra diagnostics are all like this: Not sure where that package came from or how to fix that though |
|
Ah, nice catch - can you open a separate pull request to fix that issue? |
This fixes an error that content of an error is not read if the ContentLength Header is null. Content must be read before checking the content length header. Brings the error handling in line with the success handling that also checks against 0.
I have tested this on the same code that produced the error in #26 and it now functions correctly. More tests would likely to be helpful to add to catch errors like this in future or other errors that exist with the same condition elsewhere in the code but I wouldn't know where to start with that.